-
-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
minimessage: Unify transformations and placeholders #672
Conversation
This replaces both with a new Tag system, which exposes both argumentless insertions and argument-handling transformations in one registry, with one type hierarchy.
Correctly provide the post-preprocessing location information when creating exceptions.
… just lambdas, for examinability
This allows us to mark the entire parser package as internal
.../src/main/java/net/kyori/adventure/text/minimessage/tag/resolver/TagResolverBuilderImpl.java
Show resolved
Hide resolved
...-minimessage/src/main/java/net/kyori/adventure/text/minimessage/tag/builtin/GradientTag.java
Outdated
Show resolved
Hide resolved
text-minimessage/src/main/java/net/kyori/adventure/text/minimessage/MiniMessageParser.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't review because github was broken but i noticed some stuff while testing it locally
text-minimessage/src/main/java/net/kyori/adventure/text/minimessage/MiniMessageImpl.java
Outdated
Show resolved
Hide resolved
text-minimessage/src/main/java/net/kyori/adventure/text/minimessage/MiniMessageParser.java
Show resolved
Hide resolved
text-minimessage/src/main/java/net/kyori/adventure/text/minimessage/MiniMessageParser.java
Outdated
Show resolved
Hide resolved
This simplifies error handling across arguments, and allows better state tracking in error messages down the road.
I think I've covered all the essential API elements in this PR now. I'd love to get this in by the end of the upcoming weekend, so please get your feedback in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very minor stuff.
I was a bit confused by ClickTag not implementing anything and ColorTagResolver being a tag resolver. I also not sure if I like that basically everything is static in the tags classes. Wouldn't they be tag resolvers too, just really simple ones? that would clean up the buildintags class a bit where I got a bit confused too (I guess the comment there can be discared now that I looked at the tags, lol)
like, maybe SimpleTag that is a tag resolver itself?
but overall, outstanding work, I really like how it turned out so far!
text-minimessage/src/main/java/net/kyori/adventure/text/minimessage/Context.java
Outdated
Show resolved
Hide resolved
text-minimessage/src/main/java/net/kyori/adventure/text/minimessage/MiniMessageParser.java
Show resolved
Hide resolved
text-minimessage/src/main/java/net/kyori/adventure/text/minimessage/ParsingException.java
Show resolved
Hide resolved
...-minimessage/src/main/java/net/kyori/adventure/text/minimessage/tag/builtin/BuiltInTags.java
Outdated
Show resolved
Hide resolved
...-minimessage/src/main/java/net/kyori/adventure/text/minimessage/tag/builtin/BuiltInTags.java
Outdated
Show resolved
Hide resolved
text-minimessage/src/test/java/net/kyori/adventure/text/minimessage/MiniMessageParserTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incredibly solid bit of work here - I really don't have much to comment at all beyond saying good job! I've left a few minor nitpicky comments, mainly documentation changes - but these aren't particularly blocking as I'm sure we'll clean up the actual docs upon the release of 4.10.0 which should really help users get used to this change.
Well done!
This replaces both with a new Tag system, which exposes both
argumentless insertions and argument-handling transformations in one
registry, with one type hierarchy.
I hope to open the door to piggybacking on these tag resolvers to handle serialization as well at some future point.
Still to do:
Tag
implementations, to ease debuggingContext
[ ] common impl for rainbow and gradient tagshappening later, not an API-visible change[ ] for rainbow and gradient length calculations, a way to distinguish between pre-calculated and lazily computeddelayedInserting
types (does this make sense to be restricted to just placeholders even?)ArgumentQueue
/other helpers for parsing tag arguments (something providing API along the linesargs.pop()
that auto-throws an exception if the required argument isn't present, other helpers likeTag.Argument#asDouble(): OptionalDouble
)Overview
The changes in this PR aim to unify two very similar APIs to reduce the number of concepts developers have to deal with. In doing so, I've managed to reduce some layers of abstraction, and provide the full capabilities of each system for every use case. This has resulted in some shuffling around:
Tag
sPlaceholderResolver
TransformationRegistry
TagResolver
Placeholder
TransformationFactory
TagResolver
Replacement
Transformation
Tag
As you can see, the concepts of a factory/placeholder and resolver/registry have been merged -- it is possible to consider a
Placeholder
orFactory
as a simple case of a resolver, which only contains one element.Types of
Tag
sInserting
These tags are fairly straightforward: they represent a literal
Component
. The vast majority ofTag
implementations will want to beInserting
tags. Inserting tags may also optionally be self-closing -- by default, this is only true for tags created by resolvers from thePlaceholder
class, so that placeholders are self-contained.Modifying
Modiying
tags have the ability to perform transformations on the entire component tree of their children after it has been created. This is used for the<rainbow>
and<gradient>
tags, but can be applied to any other more complex transformations.PreProcess
(orminimessage
)These tags have a value of a MiniMessage string. They are unique in that they're applied before any sort of parsing begins, so can be fragments rather than entire strings.
Examples
The implementations of the built-in tags for representing components provide a starting point for examples of how this system works. However, to address some more simple cases, and provide examples of a custom tag with richer functionality:
A simple case with placeholder-equivalent tags:
A fancier case, with custom logic
This creates a custom tag that performs all the styling needed to create a standard hyperlink using an HTML-like
a
tag: